Ok so... what is the purpose of this method? what utility does it present?

I just trying to keep query_ids from the Snowflake.
It seems useless for Airflow itself, but Hook users can use it for tracking their queries

Ok so if we keep this, i'd prefer to make the interface on DBApi more generic, like _post_execute_hook. Or something like that. Maybe there's a better name out there. Then snowflake can implement it and use it to store the query ids.

One problem is that hooks do a variety of executing in a variety of contexts. So, perhaps we can come up with something that's a little more generic, i.e. compatible with if, say, we wanted to add a hook / callback in a different context within the functionality of the hook.

Meanwhile, I'm checking with @mobuchowski to see whether we still need query ids on snowflake hook for the original purpose (which was lineage) or whether we are now scraping that info some other way.

Thanks

@dstandish yes, we're using it in OpenLineage integration: https://github.com/OpenLineage/OpenLineage/blob/67365870c932badf27ceb3ffd8df4d0d742d926c/integration/airflow/openlineage/airflow/extractors/snowflake_extractor.py#L74

Just for the record - see the k
ind of problems that the common.sql MIGHT introduce if we remove some seeminglly unused methods.

#27838

I think we should be very, very, very careful when we remove anything from common.sql provider. Because we might simply not realize that it has been used.

This is the effect of common code in full swing if you do not have very well defined "public API" upfront AND you do not check (Python WTF) that even if you declared something as protected, some other entity might rely on it.

This is the learnings we should have that once we make some code common and used elsewhere, refactoring and removing stuff from it should be done extremely carefully.

hey @potiuk i thought your position was "let's just break things" ;)

@mobuchowski it looks like you rely on such an attr on the operator class not the hook

is that correct? To me, that makes a lot more sense. Although, it looks like in main this may be broken, because I do not see anything that touches the query_ids attr on this operator after init.

@dstandish you're right, we're using it on operator - looks like it was broken on #25717.

It's not a critical for us - we can accept this breakage if we can agree on exposing this kind of information from hooks via operators in any other way. I think it would be useful for any other kinds of metadata.

For example, I dislike the fact that SQLExecuteQueryOperator gets hook in execute method, but does not set it as self instance variable.

There's a lot to be done on how to expose execution metadata.

This method still implemented here to keep Snowflake's split_statements: bool = True default parameter